Auto-detect running editor on Windows for error overlay #2552
Conversation
levrik
force-pushed
the
detect-editor-windows
branch
from
eb22140
to
adee1d0
Compare
|
Nice, thank you for working on this! Let me know when you feel it's ready for review and I can give this a spin. |
levrik
force-pushed
the
detect-editor-windows
branch
2 times, most recently
from
fc4673c
to
837ccae
Compare
|
@gaearon Ready for a first review :) Edit: AppVeyor build is broken but it seems to be not related to this PR since other PRs having the same failure. |
levrik
changed the title
levrik
force-pushed
the
detect-editor-windows
branch
from
837ccae
to
9196c7b
Compare
levrik
force-pushed
the
detect-editor-windows
branch
from
9196c7b
to
c0d2600
Compare
This sounds good to try in a separate PR if you'd like! |
| } | ||
| } | ||
| } else if (process.platform === 'win32') { | ||
| const output = child_process.execSync('powershell -Command "Get-Process | Select-Object Path"').toString(); |
There was a problem hiding this comment.
What if powershell doesn't exist on the system? Seems like this prints stuff to console. Can we silence it?
There was a problem hiding this comment.
I'll push up a fix for this
|
This looks great. Thank you so much! |
|
Fixed in 1.0.8. Please verify. |
| 'atom.exe', | ||
| 'sublime_text.exe', | ||
| 'notepad++.exe', | ||
| ]; |
There was a problem hiding this comment.
What about Visual Studio (devenv.exe)? It's the IDE for Windows. 😛
There was a problem hiding this comment.
We're taking PRs 😛
VS was my first IDE...
| } | ||
| } else if (process.platform === 'win32') { | ||
| const output = child_process | ||
| .execSync('powershell -Command "Get-Process | Select-Object Path"', { |
There was a problem hiding this comment.
It's really annoying that Node.js doesn't have built-in abstractions for this. Hopefully better FFI support should make this more reliable at least, instead of having to parse strings.
* Auto-detect running editor on Windows for error overlay * Ignore process output if powershell call fails * Support Notepad++
* Auto-detect running editor on Windows for error overlay * Ignore process output if powershell call fails * Support Notepad++
* Auto-detect running editor on Windows for error overlay * Ignore process output if powershell call fails * Support Notepad++
I decided to go with an a bit different approach for Windows than on macOS. This resolves the first two issues mentioned below.
The new approach detects only by the file name and re-uses the whole path found in the process list to launch the editor. On macOS this was not possible because some executables did not match the command needed to run (if I understood that right).
Regarding the third one:
We could improve that by launching a PowerShell at the beginning in the background and just pipe the commands in then. This would require to switch to a Promise/callback-based implementation of the whole feature. I would pick that up in a second PR later on if you're okay with it 🙂
Tested on Windows 10 with latest VS Code, Atom and Sublime Text 3.
The original content of this PR:
Also you can find the previous code here: levrik/create-react-app@adee1d0